Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(e2e tests): missing new eigenda-client required config fields #196

Merged
merged 9 commits into from
Oct 31, 2024

Conversation

samlaf
Copy link
Collaborator

@samlaf samlaf commented Oct 30, 2024

was missing ethrpc and svcmanageraddr

Also realized pull_request_target was super wrong...... reverted that commit.
pull_request_target by default checks out the target branch! so all the tests were being done against main branch and not the PR branch..

Fixes Issue

Fixes #

Changes proposed

Screenshots (Optional)

Note to reviewers

…uest_target (#153)"

This reverts commit 15b10fd.
The commit was doing things very wrong. I hadn't understood how pull_request_target works.
Was causing the workflow to run against main branch head commit instead of PR commit.
We will need to find another solution to the problem of letting external contributors run this workflow.
@samlaf samlaf requested a review from epociask October 30, 2024 00:21
epociask
epociask previously approved these changes Oct 30, 2024
e2e/setup.go Outdated Show resolved Hide resolved
@samlaf samlaf dismissed epociask’s stale review October 30, 2024 21:05

Found a regression whcih is causing the bug.. will need to update: Layr-Labs/eigenda#849

This forces test runner to be fully aware of which test suite he is running (otherwise it implicitly runs the TESTNET suite)
Previously it was printing as byte array, which is unreadable and clutters logs
TODO: will need to update this after that PR is merged
go.mod Outdated
Comment on lines 8 to 10
// TODO: merge https://github.com/Layr-Labs/eigenda/pull/849 first
// and then update this dep to the merged commit.
github.com/Layr-Labs/eigenda v0.8.5-0.20241030205712-b301232f7804
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: adf580f

@samlaf
Copy link
Collaborator Author

samlaf commented Oct 31, 2024

Merged the upstream Layr-Labs/eigenda#849, and updated eigenda dep to eigenda master head. Merging without waiting to unblock here. pull_request_target holesky-test is still failing, but that is normal because this PR changed that test, so it will no longer run after we merge to master (only the new pull_request holesky-test will run, which is passing here)

@samlaf samlaf merged commit 390e491 into main Oct 31, 2024
7 of 8 checks passed
@samlaf samlaf deleted the samlaf/fix-e2e-holesky-tests branch October 31, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants